Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Cleaned up Python sort syntax. #1663

Merged
merged 2 commits into from
Dec 8, 2021
Merged

Cleaned up Python sort syntax. #1663

merged 2 commits into from
Dec 8, 2021

Conversation

chipkent
Copy link
Member

@chipkent chipkent commented Dec 8, 2021

Cleaned up the syntax. This is ok for the v1 python API. The v2 API can take more drastic steps to achieve better syntax.

Resolves #1662

Current syntax:

from deephaven.TableTools import newTable, stringCol, intCol, doubleCol
from deephaven import SortColumn
Arrays = jpy.get_type("java.util.Arrays")
ColumnName = jpy.get_type("io.deephaven.api.ColumnName")

source = newTable(
    stringCol("Letter", "A", "D", "B", "E", "D", "A"),
    intCol("Number", 6, 7, 1, 4, 2, 4),
    doubleCol("Percent", .25, .75, .15, .80, .25, .75)
)

sort_columns = Arrays.asList(
    SortColumn.asc(ColumnName.of("Letter")),
    SortColumn.desc(ColumnName.of("Number"))
)

result= source.sort(sort_columns)

New syntax:

from deephaven.TableTools import newTable, stringCol, intCol, doubleCol
from deephaven import SortColumn, ColumnName, asList

source = newTable(
    stringCol("Letter", "A", "D", "B", "E", "D", "A"),
    intCol("Number", 6, 7, 1, 4, 2, 4),
    doubleCol("Percent", .25, .75, .15, .80, .25, .75)
)

sort_columns = asList(
    SortColumn.asc(ColumnName.of("Letter")),
    SortColumn.desc(ColumnName.of("Number"))
)

result= source.sort(sort_columns)

@chipkent chipkent added this to the Nov 2021 milestone Dec 8, 2021
@chipkent chipkent self-assigned this Dec 8, 2021
jakemulf
jakemulf requested changes Dec 8, 2021
View reviewed changes
Copy link
Contributor

@jakemulf jakemulf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small name change, otherwise looks good to me

Integrations/python/deephaven/__init__.py Outdated Show resolved Hide resolved
@chipkent @jakemulf
Reaned asList to as_list
f2a792f 
Co-authored-by: Jake Mulford <jakemulf@gmail.com>
@chipkent chipkent requested a review from jakemulf December 8, 2021 21:17
jmao-denver
jmao-denver approved these changes Dec 8, 2021
View reviewed changes
Copy link
Contributor

@jmao-denver jmao-denver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@chipkent chipkent merged commit 4b18d35 into deephaven:main Dec 8, 2021
@chipkent chipkent deleted the 1662_python_sort_syntax branch December 8, 2021 21:45
@github-actions github-actions bot locked and limited conversation to collaborators Dec 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jakemulf jakemulf jakemulf approved these changes

@jmao-denver jmao-denver jmao-denver approved these changes

Assignees

@chipkent chipkent

Labels
DocumentationNeeded python python-server-side
Projects
None yet
Milestone
Nov 2021
Development

Successfully merging this pull request may close these issues.

Python syntax for sorting needs to be more clean
3 participants
@chipkent @jmao-denver @jakemulf